[Bugfix] Allowing sqlExpression to be blank #4991
Conversation
| @@ -45,7 +45,8 @@ export default class AdhocFilter { | |||
| this.clause = adhocFilter.clause; | |||
| this.sqlExpression = null; | |||
| } else if (this.expressionType === EXPRESSION_TYPES.SQL) { | |||
| this.sqlExpression = adhocFilter.sqlExpression || | |||
There was a problem hiding this comment.
I'm not sure in what cases this would have been undefined if expressionType === sql. Gabe do you have more context here? Is checking for undefined sufficient or do I also need to check for nulls?
michellethomas
force-pushed
the
fix_filter_blank_custom_sql
branch
from
a73d29a to
6c6e2e4
Compare
| @@ -45,7 +45,8 @@ export default class AdhocFilter { | |||
| this.clause = adhocFilter.clause; | |||
| this.sqlExpression = null; | |||
There was a problem hiding this comment.
sqlExpression can be set to null here, so you can't predict whether it will be undefined or null
| @@ -45,7 +45,8 @@ export default class AdhocFilter { | |||
| this.clause = adhocFilter.clause; | |||
| this.sqlExpression = null; | |||
| } else if (this.expressionType === EXPRESSION_TYPES.SQL) { | |||
| this.sqlExpression = adhocFilter.sqlExpression || | |||
| this.sqlExpression = typeof adhocFilter.sqlExpression !== 'undefined' ? | |||
| adhocFilter.sqlExpression : | |||
| translateToSql(adhocFilter, { useSimple: true }); | |||
There was a problem hiding this comment.
what if you just removed the useSimple parameter? that might be the problem 🤔
michellethomas
force-pushed
the
fix_filter_blank_custom_sql
branch
from
6c6e2e4 to
ad4912d
Compare
Codecov Report
@@ Coverage Diff @@
## master #4991 +/- ##
======================================
Coverage 77.1% 77.1%
======================================
Files 44 44
Lines 8636 8636
======================================
Hits 6659 6659
Misses 1977 1977Continue to review full report at Codecov.
|
|
Like we discussed, I changed this to explicitly check if sqlExpression a string.
|
|
LGTM |
…ustom_sql [Bugfix] Allowing sqlExpression to be blank
If you remove the sql in the custom sql filter, you'll see a message
null undefined null.adhocFilter.sqlExpression || translateToSql(adhocFilter, { useSimple: true })was running translateToSql with simple: true on an empty string. Change it to specifically check for undefined.@GabeLoins